gh-127541: Update os.walk example #127765
Conversation
|
All commit authors signed the Contributor License Agreement. |
bedevere-app
bot
added
awaiting review
docs
|
|
||
| import os | ||
| from os.path import join, getsize | ||
| for root, dirs, files in os.walk('python/Lib/email'): | ||
| print(root, "consumes", end=" ") | ||
| print(sum(getsize(join(root, name)) for name in files), end=" ") | ||
| print("bytes in", len(files), "non-directory files") | ||
| if 'CVS' in dirs: | ||
| dirs.remove('CVS') # don't visit CVS directories | ||
| if '__pycache__' in dirs: |
There was a problem hiding this comment.
You might want to update which directory is being walked over (currently it's python/Lib/email)
There was a problem hiding this comment.
Thank you for the suggestion! I’ll update the example to use os.getcwd() for better compatibility.
There was a problem hiding this comment.
Depending where you are actually running the command, it might be very very long for the user (say they run it from $HOME). Instead, we can walk over 'python/Lib' instead.
There was a problem hiding this comment.
Thank you for the feedback! I’ve updated the example to use python/Lib as the base directory.
There was a problem hiding this comment.
What is the reason of this change?
There was a problem hiding this comment.
Since __pycache__ is more common throughout python/Lib than python/Lib/email, I updated the directory to make the example more representative and practical.
There was a problem hiding this comment.
In addition, I only have a single __pycache__ in my email package (even though email.mime exists) so you wouldn't see the "recursion" in this example I think if you compare with a find Lib/email -not -ipath '*__pycache__*'
There was a problem hiding this comment.
Starting from Lib you will walk into .git, which may be non-desirable. Lib/xml contains several subdirectories with their own __pycache__ directories.
There was a problem hiding this comment.
Ah maybe, Lib/xml is better then.
Co-authored-by: Bénédikt Tran <[email protected]>
There was a problem hiding this comment.
See my latest comment: https://github.com/python/cpython/pull/127765/files#r1876269560.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @serhiy-storchaka: please review the changes made to this pull request. |
|
|
||
| import os | ||
| from os.path import join, getsize | ||
| for root, dirs, files in os.walk('python/Lib/email'): | ||
| print(root, "consumes", end=" ") | ||
| print(sum(getsize(join(root, name)) for name in files), end=" ") | ||
| print("bytes in", len(files), "non-directory files") | ||
| if 'CVS' in dirs: | ||
| dirs.remove('CVS') # don't visit CVS directories | ||
| if '__pycache__' in dirs: |
There was a problem hiding this comment.
Starting from Lib you will walk into .git, which may be non-desirable. Lib/xml contains several subdirectories with their own __pycache__ directories.
|
Thank you! |
I updated the example to exclude
__pycache__directories instead ofCVS.While the issue suggested replacing
CVSwith.git, a comment pointed out that.gitis usually found only in the root directory, making it not equivalent toCVS, which can appear in multiple subdirectories.On the other hand,
__pycache__directories often appear across various levels of a directory tree, making them a more practical modern example!📚 Documentation preview 📚: https://cpython-previews--127765.org.readthedocs.build/